-
-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for declaring simple lambda permissions in-module #69
Conversation
FWIW, I've run a quick smoke test using my fork branch as |
c6a13d3
to
3e57260
Compare
/terratest |
5c600af
to
db40e88
Compare
@dudymas I've now added a |
/terratest |
Looking good! Now, you'll just need to make sure when
|
db40e88
to
f9171f9
Compare
@dudymas All right, I've now added |
/terratest |
unfortunately, terraform doesn't handle counts of
|
2e0df38
to
16cfd56
Compare
Oh right, the module instance itself isn't guarded by a count. Should be fixed now. |
/terratest |
1 similar comment
/terratest |
@jpalomaki could you please address this linter error:
Should be like:
|
16cfd56
to
1f67ee6
Compare
/terratest |
@dudymas @gberenice I've now fixed that linter warning, but looks like I can't run terratest myself |
/terratest |
1f67ee6
to
b1d5ba3
Compare
@gberenice I've now fixed the apparent formatting errors, please retest |
/terratest |
@jpalomaki thanks for contribution! |
These changes were released in v0.5.6. |
what
Allow lambda configuration author to optionally declare
lambda:InvokeFunction
lambda permissions directly in this module.More complex permissions configurations could still be done outside of this module.
why
This co-locates permissions related to the lambda in the module configuration (where we also declare lambda IAM role permissions), which can help a reader understand where the lambda is invoked from, e.g. in cases where the actual event sources are declared in a different root configuration.
In our specific use case, we use terragrunt to deploy the lambda function (straight from terraform registry module), so this feature would also help us avoid having to create a wrapper module just to add the necessary permission resources.
questions
lambda:InvokeFunction
and keep the number of attributes a user has to fill in, small. Does this look like a sane approach (looks like it could cover a lot of ground already, judging by examples)?for_each
is keyed by list index, which isn't ideal, since it would force recreations if items are shuffled/insertedreferences
Slack discussion, cc/ @osterman